feat(buildrunner): noop impl, poll-driven buildsignal, pipeline wiring#177
Merged
Conversation
This was referenced Jun 2, 2026
ab94b8a to
c5e5759
Compare
42a34ea to
2dd6d25
Compare
2dd6d25 to
a7d0c2a
Compare
c5e5759 to
70ddd63
Compare
a7d0c2a to
d83c3a5
Compare
70ddd63 to
55b194d
Compare
d83c3a5 to
d735e8f
Compare
55b194d to
c0fd951
Compare
d735e8f to
c368626
Compare
e298f94 to
de8932c
Compare
c368626 to
64d1a6e
Compare
de8932c to
004ca8a
Compare
64d1a6e to
1311592
Compare
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
) ## Summary ### Why? The build stage needs a vendor-agnostic abstraction for talking to a Build Runner — one that fits the existing extension family (storage, queue, conflict) and that does not have to change when the first real backend lands. Design rationale lives in `doc/rfc/build-runner.md`. ### What? Introduces the `BuildRunner` contract — three verbs, all keyed by an `entity.BuildID`: `Trigger` submits a build (ordered `base` + `head` change sets plus a free-form metadata map) and returns its ID; `Status` fetches the current `BuildStatus` and runner-defined metadata; `Cancel` requests cancellation. See `extension/buildrunner/build_runner.go` for the signatures. Highlights: - `Trigger` takes ordered `base` (assumed-good prefix from dependency batches) and `head` (changes being verified) — a runner can cache or short-circuit a prefix it has validated before, and attribute terminal failures to base vs head via `BuildMetadata`. - Build identifiers are the typed `entity.BuildID` (a lightweight `{ID string}` value, also the queue payload envelope) rather than a bare `string`, so the runner contract and the on-queue messages share one identifier type and the compiler distinguishes a build ID from other string IDs. - `metadata` is a free-form `map[string]string` for caller-supplied attributes (requester, ticket ID, trace ID) the runner MAY persist or echo back via `Status`. - `Trigger` and `Cancel` return promptly; `Status` MAY be lengthy. - The `BuildStatus` enum is narrowed to `Accepted` / `Running` / `Succeeded` / `Failed` / `Cancelled` so every backend can map its native lifecycle without leaking runner-specific stages. - `BuildMetadata` is added as a free-form map for round-tripping runner-defined attributes (build URL, duration, SHA, etc.). Naming: the interface is `BuildRunner` (not `BuildManager` — the "Manager" suffix doesn't describe what it does, and "Build" alone would collide cognitively with `entity.Build`). The package is `extension/buildrunner` to follow the repo convention used by `mergechecker`, `changeprovider`, `pusher`, `counter`, and `storage`. Implementation (noop backend, queue `PublishAfter` primitive, controller wiring, and the poll-driven `buildsignal` loop) lands in the follow-up PRs stacked on top of this one. ## Test Plan - ✅ `make test` — entity/build tests pass; mock-only consumers of the interface compile and pass. - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Stack 1. @ #175 1. #179 1. #176 1. #177
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
## Summary ### Why? Wiring tests for the orchestrator's build stage need a `BuildRunner` implementation, but real backends (BuildKite, Jenkins, etc.) won't land until later. A noop that immediately reports success is also useful as the local default in `make local-start` and as a best-case baseline where every build passes. ### What? Adds `extension/buildrunner/noop` — a `BuildRunner` that does no real work. Every `Trigger` returns a unique `noop-<n>` build ID with no error; every `Status` returns `BuildStatusSucceeded` with no metadata; `Cancel` is a no-op. The atomic counter keeps IDs unique under concurrent use. ## Test Plan - ✅ `make test` — 36 unit tests pass, including the new `extension/buildrunner/noop:noop_test` (Trigger uniqueness, Status, Cancel, interface satisfaction). - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Issues ## Stack 1. #175 1. @ #179 1. #176 1. #177
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
## Summary ### Why? The orchestrator's build poll loop needs to space `Status` calls out without consuming `retry_count` / DLQ retry slots. `Delivery.Nack` does schedule redelivery after a delay, but it overloads `retry_count` — which the operator relies on to flag real failures. The fix is a separate "postpone this work" primitive, distinct from "this delivery failed, try again", so both signals stay meaningful. ### What? Adds `Publisher.PublishAfter(ctx, topic, msg, delayMs)` to the queue extension: a fresh message inserted into the topic, made visible to subscribers only after `delayMs` from now. Distinct from `Delivery.Nack(requeueAfterMillis)`: - `Nack` is "this delivery failed, try again" — it bumps `retry_count` and eventually trips DLQ. - `PublishAfter` is "postpone this work" — `retry_count` resets to 0, DLQ stays available for true failures. SQL backing in `extension/queue/mysql`: - New `visible_after BIGINT UNSIGNED NOT NULL DEFAULT 0` column on `queue_messages`. Default 0 means immediately visible — back-compat for any existing rows. - `messageStore.InsertDelayed(ctx, topic, messages, visibleAfterMs)` is the underlying primitive. `Insert` is now a thin wrapper that passes `visibleAfterMs = 0`. - `FetchByOffset` gains a `nowMs` parameter and an `AND visible_after <= ?` predicate so subscribers skip rows whose delivery is still deferred. - `MoveToDLQ` writes `visible_after = 0` explicitly: any delay on the original message has already been consumed by the time it failed. The first consumer of `PublishAfter` is the orchestrator's poll-driven `buildsignal` loop, which lands in the stacked PR on top of this one. ## Test Plan - ✅ `bazel test //extension/queue/...` — all pass, including new coverage for `PublishAfter` (positive / zero / negative delay, closed-publisher) and for `FetchByOffset` skipping deferred rows via the new `visible_after` predicate. - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Issues ## Stack 1. #175 1. #179 1. @ #176 1. #177
## Summary
### Why?
The orchestrator's build stage needs to drive the `BuildRunner` contract
end-to-end: trigger the runner, persist the result, and poll `Status`
until terminal so the batch state machine can react. Polling has to
behave like the rest of the pipeline (queue-driven, partition-isolated,
restart-safe) rather than running as an in-process timer loop.
### What?
Stacks on top of the BuildRunner interface, noop, and `PublishAfter`
PRs. Wires the runner into the orchestrator pipeline.
The build poll loop runs as queue traffic inside the existing
`buildsignal` consumer (no separate stage). On each delivery it loads the
`Build` from storage, calls `BuildRunner.Status`, persists the result via
`BuildStore.UpdateStatus`, publishes the batch ID to `speculate` so the
state machine re-evaluates, and re-publishes itself via
`Publisher.PublishAfter` until the build reaches a terminal state. A
webhook-capable backend can publish into the same topic — the consumer
cannot tell a poll-driven message from a push.
Only the build **ID** travels on the queue (`entity.BuildID`); the
consumer reloads the full `Build` from `BuildStore`, keeping the message
small and storage the single source of truth — the same ID-on-the-queue,
load-from-storage pattern the rest of the pipeline already uses for
batches and requests. The controllers consume the runner's
`entity.BuildID` signatures (`Trigger` returns one; `Status` takes one).
Pieces:
- `orchestrator/controller/build`: assembles `base` from
`batch.Dependencies` and `head` from `batch.Contains`, calls
`Trigger`, persists the initial `Build{Accepted}` via
`BuildStore.Create` (`ErrAlreadyExists` is swallowed for redelivery),
publishes the build ID to `buildsignal`.
- `orchestrator/controller/buildsignal`: the polling consumer described
above. It loads the `Build` by ID, then polls. `PollDelayAcceptedMs=5000`,
`PollDelayRunningMs=2000` by default (vars so tests can override; a TODO
notes these should move into the `queueconfig` extension). Error
classification: only the `PublishAfter` re-schedule is wrapped retryable
(`errs.NewRetryableError`) — it is the poll loop's heartbeat, so a
transient enqueue blip nacks and replays (up to `MaxAttempts`) rather
than rejecting the loop's only live message straight to DLQ. Deserialize,
the `Build` load, `Status`, `UpdateStatus`, and the speculate publish
stay non-retryable and reject to DLQ on first failure, where an
operational republish is the recovery path.
- `example/server/orchestrator/main.go`: passes the `BuildRunner` to
both `build.NewController` and `buildsignal.NewController`; pipeline
diagram updated.
- root `BUILD.bazel`: adds `# gazelle:exclude .claude` so gazelle does
not index nested worktrees as duplicate rule definitions and corrupt
the canonical BUILD files.
## Test Plan
- ✅ `bazel test //extension/buildrunner/... //orchestrator/controller/build/... //orchestrator/controller/buildsignal/... //extension/queue/...` — all pass.
- ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean.
- ✅ `make build` — all targets compile.
- New coverage: build controller persist+publish path (with
`ErrAlreadyExists` swallow), buildsignal poll loop (terminal forwards
to speculate, non-terminal re-publishes via `PublishAfter` with
per-status delay, retryable re-publish failure, non-retryable build-load
/ `Status` / `UpdateStatus` failures reject to DLQ).
JamyDev
approved these changes
Jun 3, 2026
004ca8a to
48890aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
The orchestrator's build stage needs to drive the
BuildRunnercontractend-to-end: trigger the runner, persist the result, and poll
Statusuntil terminal so the batch state machine can react. Polling has to
behave like the rest of the pipeline (queue-driven, partition-isolated,
restart-safe) rather than running as an in-process timer loop.
What?
Stacks on top of the BuildRunner interface, noop, and
PublishAfterPRs. Wires the runner into the orchestrator pipeline.
The build poll loop runs as queue traffic inside the existing
buildsignalconsumer (no separate stage). On each delivery it loads theBuildfrom storage, callsBuildRunner.Status, persists the result viaBuildStore.UpdateStatus, publishes the batch ID tospeculateso thestate machine re-evaluates, and re-publishes itself via
Publisher.PublishAfteruntil the build reaches a terminal state. Awebhook-capable backend can publish into the same topic — the consumer
cannot tell a poll-driven message from a push.
Only the build ID travels on the queue (
entity.BuildID); theconsumer reloads the full
BuildfromBuildStore, keeping the messagesmall and storage the single source of truth — the same ID-on-the-queue,
load-from-storage pattern the rest of the pipeline already uses for
batches and requests. The controllers consume the runner's
entity.BuildIDsignatures (Triggerreturns one;Statustakes one).Pieces:
orchestrator/controller/build: assemblesbasefrombatch.Dependenciesandheadfrombatch.Contains, callsTrigger, persists the initialBuild{Accepted}viaBuildStore.Create(ErrAlreadyExistsis swallowed for redelivery),publishes the build ID to
buildsignal.orchestrator/controller/buildsignal: the polling consumer describedabove. It loads the
Buildby ID, then polls.PollDelayAcceptedMs=5000,PollDelayRunningMs=2000by default (vars so tests can override; a TODOnotes these should move into the
queueconfigextension). Errorclassification: only the
PublishAfterre-schedule is wrapped retryable(
errs.NewRetryableError) — it is the poll loop's heartbeat, so atransient enqueue blip nacks and replays (up to
MaxAttempts) ratherthan rejecting the loop's only live message straight to DLQ. Deserialize,
the
Buildload,Status,UpdateStatus, and the speculate publishstay non-retryable and reject to DLQ on first failure, where an
operational republish is the recovery path.
example/server/orchestrator/main.go: passes theBuildRunnertoboth
build.NewControllerandbuildsignal.NewController; pipelinediagram updated.
BUILD.bazel: adds# gazelle:exclude .claudeso gazelle doesnot index nested worktrees as duplicate rule definitions and corrupt
the canonical BUILD files.
Test Plan
bazel test //extension/buildrunner/... //orchestrator/controller/build/... //orchestrator/controller/buildsignal/... //extension/queue/...— all pass.make fmt lint check-tidy check-gazelle check-mocks— clean.make build— all targets compile.ErrAlreadyExistsswallow), buildsignal poll loop (terminal forwardsto speculate, non-terminal re-publishes via
PublishAfterwithper-status delay, retryable re-publish failure, non-retryable build-load
/
Status/UpdateStatusfailures reject to DLQ).